iterativerobotpy.py and timedrobotpy.py #161
Open
+2,152
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
virtuald
reviewed
Apr 6, 2025
auscompgeek
reviewed
Apr 6, 2025
|
Top level questions: Is the filename right? Is the file in the right spot in the tree? Is the class name right? What are the appropriate tests? What happens if someone deletes the watchdog? Is the there a higher level watchdog that will stop the motors? Can somebody figure out how to call the shuffleboard? |
|
I'm not sure about the filename/classname yet. I think you're approaching it as an alternative, but I sorta want a replacement? But, I guess we could go either way. TBH, before deciding on alternative/replacement, probably need someone to do some testing on a real robot to see if it makes loop overrun issues more/less/same.
... yeah, the only real way to test this is likely going to be by testing examples, which is something I'd like to do but haven't taken the time to do (see #154). Java has tests for TimedRobot, but they basically do the same thing that the pyfrc test framework does (maybe that's an argument for moving some of that into here...).
If someone deletes the watchdog, that's their problem -- but also, the motor watchdog is actually a separate thing? This one just tracks loop overruns and tells the user about it.
|
|
This version simulates a TimedRobot, advances time, logs to AdvantageScope in a reasonable way, but does not move the robot in simulation. It feels like some sort of enable interlock on the simulator is missing. Generally all status results from hal.xxx calls seem to result in a strange value, but the calls seem to work.
do not seem to make the pure python files editable in place. Could use some detailed advice on how to get the python files to be editable in place. |
|
This is the code I'm using to simulate: https://github.com/spiresfrc9106/spiresRobot2025/blob/exp-timedrobotpy/robot.py#L46 |
|
For |
|
It's hard to tell which libraries need to be uninstalled for mostrobotpy and which need to stay for robotpy: t2.txt: |
|
The error you're getting has to do with a quirk of how macOS support is implemented. What it comes down to is that you need to have all robotpy wrapper dependencies installed the same way (either via develop or installed normally). If you mix it then you sometimes get weird errors like that. Typically, I use virtualenv (and specifically, virtualenvwrapper) to manage my various python development environments. I don't recommend that normal RobotPy users use virtualenvs unless they're comfortable with them, but they're really useful for python development -- and because they're effectively disposable, you can do a lot of weird changes in one environment without worrying too much about how much you're breaking. Specifically, I have different environments set up for installation from pypi, development, and various branches that I might be working on. If you were using a virtualenv, I would just say delete your virtualenv and start installing packages from scratch. If you're not using a virtualenv, then just uninstall all robotpy-* packages and wpilib and pyntcore, and then run develop again. |
|
I normally use When I re-ran that in an empty My conclusion is the More basic though is that https://github.com/robotpy/mostrobotpy/blob/main/README.md should define the relationship (what they do and where they are and how to find instructions to build/install them) between pc Also more basic is that https://github.com/robotpy/mostrobotpy/blob/main/README.md should define when to run:
and and and the command to then the installation instructions should take one all the way to a |
Yes and no. As mentioned above:
What this means is that if you are installing the base robotpy packages in develop mode, you must also install robotpy-rev, ctre, etc in develop mode. To install robotpy-rev, you would need to There might be a way out of the macOS problem by messing with the library name, but I haven't had time to look deeper into it. I agree there should be more documentation. I also agree the installer scripts should be better to make this easier. |
|
I have been trying lots of different paths to build mostrobotpy editable on OSX, Debian, and Windows, on a full production robot code base in simulation. Goal is to be able to run on real robot code. (I'm typing not cut and pasting so there may be some typo's here) I end up with two copies of many of the repos needed in the system: I can uninstall those with But then I need to install a specific version of photonlibpy: which will then also uninstalls the editable development parts of robotpy Note that I have been automating these steps here: https://github.com/MikeStitt/pythonExperiments/tree/main/tryRobotPyBuildEditable But as I search online for ways to tell This seems to be the same on debian, and osx, (I haven't got my windows build to complete yet). Thoughts on a valid development/debugging/testing installation process? -Mike |
|
The multiple package problem is likely caused by pip. I note that you have 23.1 installed, try upgrading. The version numbers for your editable installs are photonlibpy has a pinned dependency on Telling pip to ignore dependencies (via |
|
One last note, because I don't trust setuptools to do the right thing, I run We will not be using setuptools next year, so hopefully some of that will go away. |
|
My windows builds using x64 Native Tools Command Prompt fail this way: |
|
That error seems really familiar, I think we ran into it last year? I don't use Windows at all these days. I think you need to upgrade Visual Studio... according to this CD post you need 17.9 or later? |
|
This feels like a silly question, really bad planning on my part. But in debian bookworm I get: Debian bookworm doesn't support robotpy sim? |
|
WPILib does not support simulation on linux/aarch64 |
|
Actually, that doesn't seem to be true anymore, https://frcmaven.wpi.edu/ui/native/release/edu/wpi/first/halsim/halsim_gui/2025.3.2/ shows aarch64 packages. We would need to update mostrobotpy to build them and update the robotpy-meta constraints. |
|
I thought we do build it, it's just that by default robotpy-meta doesn't install the sim stuff on Linux aarch64? |
|
I have found a route to simulate a full robot with mostrobotpy editable on a OSX M3 Mac: First install Then: Python click module does most of the heavy lifting: Which runs these commands: Then: Things that were not obvious were:
|
Simulation should work fine on Linux aarch64. I've run the simulation for the examples on my Raspberry Pi 400. |
I created this issue to document where we are currently at: robotpy/robotpy-meta#33 |
|
Getting pretty close to the final product. It feels like I've resolved most of the review comments. Adding the docstrings uncovered some new conversations we should have about consistency in the API. The remaining tasks include: resolve the remaining todo's in the code, add unit tests, perhaps functional tests, test to see how how much slower the python version is, and test as a command2 TimedCommandRobot. @virtuald @auscompgeek, please look at the todo's in the code. |
virtuald
reviewed
Apr 30, 2025
virtuald
reviewed
Apr 30, 2025
|
Now that this test passes on my machine: subprojects/robotpy-wpilib/tests/test_timedrobot.py which just tests the scheduling math to schedule the future periodic callbacks, I'm fairly confident that the code is pretty close to working correctly. Perhaps @virtuald or @auscompgeek have suggestions on how to connect existing functional tests to this code? |
|
Well. There really aren't any python functional tests for this -- we're pretty light on testing since the wrapper stuff tends to Just Work if it compiles. The examples repo does full-robot tests -- eventually I'd like to run those tests as part of the CI for this repo, but haven't gotten around to it. It would probably make sense to port the tests in allwpilib from Java or C++ to python, assuming they exist. ChatGPT might give you a good first translation of them. |
|
Is there a spot in the existing robotpy code base that is equivalent to this line? In the existing robotpy code base is the thread a c++ thread or a python thread? I’m hoping to not have pave new ground in the threading model of the new test. |
|
Okay I will rebase on the current main. |
|
How would I install the artifacts from github the build: 1 - In MacOs? |
|
One thing I have noticed with |
|
I've pushed releases for rev, navx, urcl, and playingwithfusion -- so all of those should be available on PyPI within the next 20 minutes or so. To install releases from github, you can download the zip file from the action and unzip it somewhere. Then use Installation on RoboRIO is a bit more tricky because it depends on the robotpy package which enforces certain versions, but I should be able to push a robotpy release tonight. In theory, you should be able to copy the wheels to |
|
I haven't looked very hard at how TimedRobot works, so it's not clear to me whether that is expected behavior or not. I would assume that the timing is arbitrary, but I could see how one might think otherwise. |
MikeStitt
changed the title
MikeStitt
force-pushed
the
pyIterativeAndTimedRobot
branch
from
3eccb59 to
14c4096
Compare
|
@virtuald looking at I picked off these commands: It crashes here: Which is strange, because it seems like the online builds are working. |
|
I also tried this: |
|
Could use a suggestion as to what the best build editable process is now. |
|
You need to Additionally, due to pypackaging-native/pkgconf-pypi#70, you should do |
|
@virtuald Thanks. This now works. |
|
@virtuald this doesn't work yet: results in: |
|
Tracking the rdev issues at #172 |
|
Documenting a path that does allow me to these steps work: This didn't work because This didn't work because This didn't work because This works because This works because robotpy sim now uses the editable For reference |
|
I have this simulateable That can simulate with the C++ The With the legacy c++ Order of magnitude, it takes 81.28 microseconds to get to the first added task. and 91.37 microseconds to get to the 10th one. Switching to Order of magnitude the python On the MacBook Pro, concentrating on the time to get to the first extra task, the python version is, order of magnitude, 50 microseconds slower. If we can figure out how to run this code on a RoboRio, and the results are reasonable, I feel like this task is getting close to being complete. |
|
Next steps: |
|
@virtuald Could you look at https://github.com/robotpy/mostrobotpy/actions/runs/15209970105 ? The failure doesn't seem to relate to my changes. |
|
I was able to do some timing measurements on my roborio 2 by copying Order of magnitude, |
MikeStitt
force-pushed
the
pyIterativeAndTimedRobot
branch
from
9b71587 to
3f3d25e
Compare
Signed-off-by: Mike Stitt <mike@stitt.cc>
MikeStitt
force-pushed
the
pyIterativeAndTimedRobot
branch
from
fcfd3ac to
302d070
Compare
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
|
Documenting the steps to sim: Results: Comparing to a simulation of c++ |
|
Compared to c++ Order of magnitude, 750-420, To install the libraries, I unzipped: Then: |
Signed-off-by: Mike Stitt <mike@stitt.cc>
|
@virtuald @auscompgeek I'm at the stage where I'm running out of next steps on this pull request. Perhaps you could look it over again. |
auscompgeek
reviewed
May 29, 2025
| return f"{{func={self.func.__name__}, _periodUs={self._periodUs}, expirationUs={self.expirationUs}}}" | ||
|
|
||
|
|
||
| class _OrderedListSort: |
Comment on lines
+21
to
+22
| _kResourceType_Framework = tResourceType.kResourceType_Framework | ||
| _kFramework_Timed = tInstances.kFramework_Timed |
There was a problem hiding this comment.
This seems like an excessive over-optimisation. These constants are only used once at robot start. Putting this here makes all robot code pay the cost of reading this, even if they're not using TimedRobot.
|
|
||
| from .iterativerobotpy import IterativeRobotPy | ||
|
|
||
| _getFPGATime = RobotController.getFPGATime |
There was a problem hiding this comment.
I think putting this in the robot instance, to avoid the global lookup, will probably be faster?
| return True | ||
|
|
||
| def __repr__(self) -> str: | ||
| return f"{{func={self.func.__name__}, _periodUs={self._periodUs}, expirationUs={self.expirationUs}}}" |
There was a problem hiding this comment.
This should probably just be a dataclass, rather than reimplementing __repr__ by hand?
| if self.isTestEnabled(): | ||
| raise RuntimeError("Can't configure test mode while in test mode!") | ||
| if not self._reportedLw and testLW: | ||
| report(_kResourceType_SmartDashboard, _kSmartDashboard_LiveWindow) |
There was a problem hiding this comment.
This method shouldn't be on the critical path for the control loop, so no need to try to optimise accessing globals here.
| _kResourceType_Framework = tResourceType.kResourceType_Framework | ||
| _kFramework_Timed = tInstances.kFramework_Timed | ||
|
|
||
| microsecondsAsInt = int |
There was a problem hiding this comment.
Suggested change
| microsecondsAsInt = int | |
| microseconds: TypeAlias = int |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A draft to get progress going.